gh-94912: Adjusted check for non-standard coroutine function marker.#100935
Merged
gvanrossum merged 1 commit intopython:mainfrom Jan 11, 2023
Merged
gh-94912: Adjusted check for non-standard coroutine function marker.#100935gvanrossum merged 1 commit intopython:mainfrom
gvanrossum merged 1 commit intopython:mainfrom
Conversation
…rker. The initial implementation did not correctly identify explicitly marked class instances. Follow up to 532aa4e
Contributor
Author
|
I think this could skip news? 🤔 |
Contributor
|
Confirming this resolves the issues for Django ✔️ Thanks! |
sobolevn
reviewed
Jan 11, 2023
| while ismethod(f): | ||
| f = f.__func__ | ||
| f = functools._unwrap_partial(f) | ||
| if not (isfunction(f) or _signature_is_functionlike(f)): |
Member
There was a problem hiding this comment.
What else can we mark as coroutine-like? Functions, methods, other callables like partial, classes. Anything else?
Contributor
Author
There was a problem hiding this comment.
Hi @sobolevn. I'm not sure I follow 🤔
The cases that have occurred up to Python 3.11, using the old asyncio version, have been callable classes and sync functions. Those are (now) covered. We also added tests for static and class method and lambdas (but I've not seen anyone using those).
The other case that would simplify client code is automatic detection of the __call__ method (either async def or marked) but I think the preference is not to add that.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The initial implementation did not correctly identify explicitly marked class instances.
Follow up to 532aa4e from #99247
(Bug in Python 3.12.0a4)
Integrating Python 3.12.0a4 with the Django test suite we see that the
_has_coroutine_mark()is not allowing the key case of a class being marked as an asynchronous callable. (This is my fault: in the discussion with @gvanrossum on #99247 over whether we'd cover the marked andasync def__call__cases automatically, I forgot to add the explicit test for the marked instances.)I note the
isfunction(f) or _signature_is_functionlike(f)restriction is untested, and not, I think, necessary. (If folks appliedmarkcoroutinefunction, I think we have to assume that they know what they're up to.)asyncio.iscoroutinefunctiona deprecated alias ofinspect.iscoroutinefunctionand removeasyncio.coroutines._is_coroutine#94912